Repository navigation
add host passphrase entry and typed device states - #137
benma-agent wants to merge 5 commits into
Conversation
The sequence counter is incremented after writing each continuation frame. After frame 127 it becomes 128, causing the encoder to reject a valid completed message. This affects payloads of 7,551 to 7,609 bytes. Only report too many frames if there is still payload left to encode.
The encoder casts message lengths to u16 before checking the required buffer size. Lengths above u16::MAX wrap, making that check unreliable; other oversized messages are only rejected later by buffer or frame limits. Reject payloads above the 7,609-byte U2F HID limit before narrowing the length or writing any frames.
U2fHidCommunication::write unconditionally unwraps the framing result. If a message is too large for U2F HID, encoding returns an error and the library panics instead of returning a transport error. Add U2fEncode and propagate encoding failures through ReadWrite::write.
Introduce a PairedBitBox enum with locked, unlocked and uninitialized variants backed by a private session. Expose wallet operations only on UnlockedBitBox, and consume UninitializedBitBox when restoring a wallet. Keep device entry and unlock before pairing on every firmware version. Validate the legacy unlock response to distinguish an unlocked wallet from an uninitialized device. Reserve LockedBitBox::unlock for a later transition; the connection flow cannot return that state yet. Adapt existing examples and simulator helpers to the typed states.
|
This is a breaking API change, but this library is on version |
Use paired protobuf unlock on firmware v9.28.0 and later, returning LockedBitBox after pairing and completing the transition through LockedBitBox::unlock. Keep legacy device entry on older firmware. Add PassphraseConfig with asynchronous input and optional availability callbacks. Integrations can offer a host-entry button while device entry is available, or request host entry automatically. Invoke the input callback only after approval on the device. Expire stale request handles and reset the session when unlocking fails. Copy the firmware protobuf definitions and regenerate the Rust bindings. Add a simple automatic CLI example and a Cursive example with a host button.
205c76e to
7f503d8
Compare
beerosagos-agent
left a comment
beerosagos-agent
left a comment
There was a problem hiding this comment.
Reviewed 0663f04..7f503d8 for correctness and security, followed by a separate simplification pass. The inline comments cover one confirmed API regression and two non-blocking simplification nits. I found no confirmed security flaw in the changed paths.
Validation passed: 47 normal unit tests, 46 multithreaded unit tests, two compile-fail doctests, and four device tests across all 11 configured firmware versions. Formatting and example compilation also passed. I reproduced the setup regression on the v9.27.1 simulator and tested the simplification candidates in a temporary copy, including all legal U2F payload lengths, undersized buffers, oversized inputs, both threading configurations, and device tests. The v9.28 host-entry flow was checked against firmware source and unit tests, without a hardware run.
| /// Show recovery words on the Bitbox. | ||
| pub async fn show_mnemonic(&self) -> Result<(), Error> { | ||
| let _api_call = self.begin_api_call().await; | ||
| let _api_call = self.session.begin_api_call().await; |
There was a problem hiding this comment.
[P2] Preserve the setup form of show_mnemonic()
Moving this operation exclusively onto UnlockedBitBox removes a valid setup flow. Firmware also accepts ShowMnemonic in its Seeded state: the seed has been created, but its backup has not yet been completed. If another client starts setup and the device is then reconnected here without being powered off, wait_confirm() returns UninitializedBitBox, which has no method to show those recovery words and finish initialization. The previous public API could perform this operation.
I reproduced this on the v9.27.1 simulator: create a seed with SetPassword, reconnect with initialized=false, then successfully send ShowMnemonic through the private session. The device reports initialized=true afterward. The firmware supports the operation; the new public Rust type prevents calling it.
Please expose the setup form on UninitializedBitBox, transitioning to UnlockedBitBox after successful confirmation. This preserves the typed-state design while retaining the firmware's valid setup transition.
|
|
||
| impl<R: Runtime> Session<R> { | ||
| async fn unlock(&self, config: &PassphraseConfig) -> Result<(), Error> { | ||
| let _api_call = self.begin_api_call().await; |
There was a problem hiding this comment.
Non-blocking simplification: omit the mutex acquisition in consuming transitions
The only caller of this helper is LockedBitBox::unlock(self, ...), which consumes the device. The session is private and cannot have concurrent callers at that point, so this lock cannot contend. The same applies to the lock in UninitializedBitBox::restore_from_mnemonic(self).
Removing these two acquisitions preserves errors, cancellation cleanup, protocol timing, and thread safety; the locks are immediately available under the existing ownership guarantees. Keep serialization for operations accepting shared references, where concurrent calls are possible.
I tested removal in a temporary copy with both threading configurations and device tests; all passed, including unlock error/cancellation and future-Send coverage.
|
|
||
| seq += 1; | ||
| if seq > 127 { | ||
| if seq > 127 && !message.is_empty() { |
There was a problem hiding this comment.
Non-blocking simplification: remove the now-unreachable continuation-count check
The new MAX_PAYLOAD_LEN check at the start of encode() already limits the payload to 7,609 bytes: 57 bytes in the initial frame plus at most 128 continuation frames of 59 bytes. Consequently, when seq reaches 128, no payload can remain, and this branch cannot be taken.
Removing it preserves accepted payloads and error behavior: oversized messages still fail the initial validation, and insufficient output buffers still fail the existing buffer-length check. No protocol, compatibility, or security behavior changes.
I verified the simplified encoder with round trips for every legal payload length, undersized output buffers for each length, and oversized lengths including 65,535, 65,536, and 65,537 bytes.
Add configurable host passphrase entry for firmware v9.28.0 and later,
supporting automatic entry or an optional host-entry button. Prompt for the
passphrase only after device approval, and keep legacy unlocking for older
firmware.
Introduce
PairedBitBox::{Locked, Unlocked, Uninitialized}so wallet operationsare only available on
UnlockedBitBox. This changeswait_confirm()to returnthe appropriate state; initialized devices on newer firmware use an explicit
LockedBitBox::unlock()after pairing.Includes copied firmware protobufs, simple CLI and Cursive examples, and
separate U2F fixes for the final continuation frame, payload size checking,
and encoding error propagation.